fixed messages are repeated on page refresh & process queue only when client is leader #2703

parasharrajat · 2021-05-05T19:42:56Z

Please review @marcaaron.

Details

This PR tries to fix the duplicate request issue from the Web platform.
On the web, a user can open multiple tabs for E.cash. And all of these clients are able to send requests.
But the User can go offline any time. we have a mechanism to handle requests when the user is offline.
Currently there is an issue that these persisted requests are duplicated when the user comes back online.
Each client(tab) has an in-memory network queue. and We have global NETWORK_REQUEST_QUEUE for persisted requests.
When a user is offline all the clients will push their requests to an in-memory network queue, from which we filter all the retriable requests and push those to our persisted queue. We filter the in-memory queue and remove these persisted requests that are stored in the persisted queue.
Due to Onyx, this persisted queue is synced across the clients.
When the user comes back online, we ask the leader client to process all the persisted requests from each client. Thus no request will be duplicated. After all the requests are sent to the server, messages will be sync across tabs via Onyx, keeping the same report actions for all clients.

Fixed Issues

Fixes #2451
Fixes #4214

Tests / QA Steps

Log in to staging.expensify.cash
Navigate to a conversation
Disable the internet connection
Send a message while offline
Enable the internet connection and immediately refresh the page.
Messages should not be repeated.

Tested On

Screenshots

Web

Before

OFFLINE.mp4

After

new.mp4

…active client

parasharrajat · 2021-05-05T19:50:22Z

src/libs/ActiveClientManager/index.js

+let isInit = false;
+
+// Are we ready to determine active Client?
+const isReady = new Promise((res) => {


These changes are needed as the Client manager incorrectly reports the leadership of the client

Can you maybe elaborate on this?

Yeah sure. There are cases where we run important callbacks once but It is possible that the client is not set before that callback call. Thus a promise to make sure that we run the callback. We have to use a flag and combination of Promises to achieve this.
as

function init() { Onyx.merge(ONYXKEYS.ACTIVE_CLIENTS, [clientID]); isInit = true; }

The above method does not really initialize the client it just request Onyx to set the client id. merge Ops is async and I think it does not return thenable callback thus we have to go to connect call.

if (isInit) { setReady(); }

Now we mark is ready by resolving the promise as we have set the clientId on the client stack. This connect could fire multiple times, we only resolve the promise when isInit is set.

Would it be possible to wait until the client is set to do the rest of the logic here?

One idea would be to add an onReady function to ActiveClientsManager so that we can register a callback then wait to do the Onyx.connect() below?

I think it might make it more obvious that we are waiting for the active client to be set

ActiveClientsManager.onReady(() => { Onyx.connect({ key: ONYXKEYS.NETWORK_REQUEST_QUEUE, callback: ... }) });

Ok Thanks I am seeing the problem now. And yeah, Onyx being asynchronous is making this difficult once again 😓

marcaaron

Hey @parasharrajat, Can we try to document this code better and add some comments? Overall, I'm confused by the implementation and unsure why we're doing most of what we're doing.

marcaaron · 2021-05-07T00:04:11Z

src/libs/ActiveClientManager/index.js

+let isInit = false;
+
+// Are we ready to determine active Client?
+const isReady = new Promise((res) => {


Can you maybe elaborate on this?

src/libs/ActiveClientManager/index.js

marcaaron · 2021-05-07T00:08:49Z

src/libs/Network.js


 let isQueuePaused = false;

+// Are we retrying requests when user comes back online?
+let retryRequestsAdded = false;


The comment above doesn't really help explain what the code is doing or how it works.

src/libs/Network.js

marcaaron · 2021-05-07T00:09:37Z

src/libs/Network.js

-            return;
-        }
+        ActiveClientManager.isReady.then(() => {
+            // Only process queue when client is Leader and we have not retried the requests already


Why should we only process the queue when the client is the leader? (Put it in the comment)

marcaaron · 2021-05-07T00:11:09Z

src/libs/Network.js

@@ -112,6 +122,7 @@ function processNetworkRequestQueue() {
            !request.data.doNotRetry && request.data.persist
        ));
        Onyx.set(ONYXKEYS.NETWORK_REQUEST_QUEUE, retryableRequests);
+        retryRequestsAdded = true;


Why do we need to set this flag?

@marcaaron. We have a queue of requests as networkRequestQueue. we take retry requests and save them in ONYXKEYS.NETWORK_REQUEST_QUEUE in case the user closes the tab or refreshes it. Now let's say the user's internet is back but he didn't refresh the tab so networkRequestQueue will continue processing all those requests. This flag records this behavior. so when this is true we know we have processed the requests and clear the requests.

// User came back online and the request queue will resume, thus we should clear the NETWORK_REQUEST_QUEUE if (ActiveClientManager.isClientTheLeader() && retryRequestsAdded) { Onyx.set(ONYXKEYS.NETWORK_REQUEST_QUEUE, []); }

Also Onyx.set(ONYXKEYS.NETWORK_REQUEST_QUEUE, retryableRequests);. We reset the queue here but what will happen if the user is sending requests from multiple tabs. We will overwrite the old requests. Do we want this to behave like this or we should merge those requests Onyx.merge?

marcaaron · 2021-05-07T00:17:09Z

src/libs/Network.js

@@ -160,6 +171,11 @@ function processNetworkRequestQueue() {
            .catch(error => onError(queuedRequest, error));
    });

+    // User came back online and the request queue will resume, thus we should clear the NETWORK_REQUEST_QUEUE
+    if (ActiveClientManager.isClientTheLeader() && retryRequestsAdded) {


I'm still a little lost on why we only want to clear the queue when the client is the leader and there is also no explanation about the significance of retryRequestsAdded. Can you try explaining this better in the comments?

// User came back online

How exactly does this tell us the user came back online?

So When a user goes offline we filter the network request queue and put the retrievable requests on persisted Queue. But it is possible that a user can come back online without refreshing the page. Now those retriable requests will process as a part of networkRequestQueue. Thus we don't want to process the persisted queue ONYXKEYS.NETWORK_REQUEST_QUEUE. It should only run when the user refresh the page.

We only allow the leader to handle the ONYXKEYS.NETWORK_REQUEST_QUEUE as persisted requests shared by tabs and make sure that we only clear the queue when they were processed via normal networkRequestQueue. As we have processed the normal queue, now we will not need them to be persisted thus clear them.

Ok, it's still a little unclear to me, but I might have to just look harder at this and try to offer some better suggestions.

parasharrajat · 2021-05-07T13:23:35Z

I will comment on everything as requested.

marcaaron · 2021-05-07T17:19:23Z

src/libs/ActiveClientManager/index.js

+let isInit = false;
+
+// Are we ready to determine active Client?
+const isReady = new Promise((res) => {


Would it be possible to wait until the client is set to do the rest of the logic here?

One idea would be to add an onReady function to ActiveClientsManager so that we can register a callback then wait to do the Onyx.connect() below?

I think it might make it more obvious that we are waiting for the active client to be set

ActiveClientsManager.onReady(() => { Onyx.connect({ key: ONYXKEYS.NETWORK_REQUEST_QUEUE, callback: ... }) });

marcaaron · 2021-05-07T17:24:12Z

src/libs/ActiveClientManager/index.js

+let setReady = null;
+
+// Whether the client Manager has started
+let isInit = false;


Instead could we have something like:

shouldFireOnReadyCallback and didInitialize and then do

if (shouldFireOnReadyCallback && !didInitialize) { onReadyCallback(); didInitialize = true; }

marcaaron · 2021-05-07T17:24:45Z

src/libs/ActiveClientManager/index.js

+let isInit = false;
+
+// Are we ready to determine active Client?
+const isReady = new Promise((res) => {


Ok Thanks I am seeing the problem now. And yeah, Onyx being asynchronous is making this difficult once again 😓

src/libs/Network.js

marcaaron · 2021-05-07T17:27:54Z

src/libs/Network.js

@@ -160,6 +171,11 @@ function processNetworkRequestQueue() {
            .catch(error => onError(queuedRequest, error));
    });

+    // User came back online and the request queue will resume, thus we should clear the NETWORK_REQUEST_QUEUE
+    if (ActiveClientManager.isClientTheLeader() && retryRequestsAdded) {


Ok, it's still a little unclear to me, but I might have to just look harder at this and try to offer some better suggestions.

…asharrajat/offline

parasharrajat · 2021-05-10T16:17:12Z

@marcaaron So I found one more side effect so I changed the logic completely.

Now the code completely rely on persisted queue. let me know what do you think of it. I would be happy to explain everything.

Due to Onyx.connect is called when user comes back online, we can just use the persisted queue and new logic is build up on that.

parasharrajat · 2021-05-10T16:55:19Z

src/libs/ActiveClientManager/index.js

+ *
+ * @returns {Promise<void>}
+ */
+function isReady() {


This way we don't need to keep the promise in memory. I used Promise over your suggested callback method onReady(() => {}). We can just use promise in multiple places for multiple tasks. The callback would require an array to be implemented for that purpose.

Ok, I want to do it the way I've suggested for consistency reasons. It's not that your idea won't work, it's that we don't have many patterns like this and it's pretty unusual to see.

We can just use promise in multiple places for multiple tasks. The callback would require an array to be implemented for that purpose.

There is no need to do this as only one thing needs to know whether we have "initialized".

marcaaron · 2021-05-10T17:09:29Z

src/libs/ActiveClientManager/index.js

+ *
+ * @returns {Promise<void>}
+ */
+function isReady() {


Ok, I want to do it the way I've suggested for consistency reasons. It's not that your idea won't work, it's that we don't have many patterns like this and it's pretty unusual to see.

We can just use promise in multiple places for multiple tasks. The callback would require an array to be implemented for that purpose.

There is no need to do this as only one thing needs to know whether we have "initialized".

marcaaron · 2021-05-10T17:12:00Z

src/libs/Network.js

-        if (didLoadPersistedRequests || !persistedRequests) {
-            return;
-        }
+        ActiveClientManager.isReady().then(() => {


Sorry, I will repeat that I would prefer that we not do the check this way.

marcaaron · 2021-05-10T17:17:11Z

src/libs/Network.js

+
+        // If we have a request then we need to check if it can be persisted in case we close the tab while offline.
+        // We filter persisted requests from the normal Queue to remove duplicates
+        networkRequestQueue = _.reject(networkRequestQueue, (request) => {


We filter persisted requests from the normal Queue to remove duplicates

Which duplicates are we talking about here? Can you provide an example? It's hard to visualize what you mean or why we are removing these from the normal request queue in memory.

Wouldn't this mean that if I add a comment while offline that it will be "removed" from the queue and not sent again when I come back online?

So Onyx.connect is called for NETWORK_REQUEST_QUEUE when we come back online which takes care of those.

Can you link me to where this happens?

When a user comes online on line 37 we process the queue by fake merge which I will optimize based on the suggestion.

src/libs/Network.js

parasharrajat · 2021-05-10T17:52:40Z

Sorry. Posting it here.

I found the following issues.

Messages that were repeated were on the same tab. (The first solution fix this part)
I test messages from multiple tabs. But it was not covered so I pushed a new implementation that covers this as well. I think this one is simpler ad more clear.

What the code is doing?

OLD Architecture

####Single Tab(Client) case

When a user is offline and we get requests, We take retry requests from the normal queue and push them to Persisted Queue(We have to). We keep the copy of those requests in the normal queue as well. Thus when we get back online. requests will be sent from Persisted Queue & normal queue.(duplicates)

Multi Client Case

We get requests while offline, Single Tab(Client) case will be repeated for all tabs. But Persisted Queue is shared among tabs so we have multi-tab duplicated requests now. With respect to this, due to when we push to Persisted queue. Onyx.connect will be called and it will push the requests to each tabs normal queue for the first time then didLoadPersistedRequests will block further calls.

So Recent changes change the architecture in the following way

Proposed Architecture

We receive requests when the user is offline, we push retry requests to persisted queue and remove them from the normal queue. Now Onyx.connect will be called for all tabs but isOffline check will block that. For all tabs same behaviour will happen. and Persisted queue has single instance of all the requests from all tabs.
Now the internet is back, Onyx.connect is called for all tabs but only the leader client will push those requests in the networkqueue thus no duplicates.

OR

If the user closed the tab, we still have those requests on Persisted Queue, Onyx.connect will be called for all tabs, Again the only leader will process them.

I think it is a simpler approach and more cleaner but I could be missing something. Otherwise, I don't think fixing the linked issue considering single tab behavior is good it would be temporary. We can wait until we are looking at the whole scenario.

marcaaron · 2021-06-07T18:19:21Z

Hey @parasharrajat can you please summarize the status of this PR and linked issue? Thanks.

parasharrajat · 2021-06-08T16:38:57Z

I will look into again and try to cross check everything. Also I will update the Promise to what you have suggested.

Overall, my solution would be based on the explanation above.

joelbettner · 2021-06-18T20:37:19Z

gentle bump

parasharrajat · 2021-06-18T20:42:03Z

Thanks for the Bump. I will try to check it off from my list this weekend. I need to rework here as there have been numerous changes since the PR was created.

parasharrajat · 2021-06-19T22:31:46Z

@joelbettner I have updated the PR and tested it.
@marcaaron I have a concern about your request of using callback instead of Promise to check ActiveClientManager readiness.
There are two scenes.
It is not deterministic when the client will become active. It is possible that the client could be ready already when we register a callback. or another case would be, that client is not yet ready when we register the callback.

For the first case, we need to fire the callback as soon as we register. thus it will require another flag. I believe that the promise-based approach is neater and it allows multiple handlers. Let me know if you want me to do it in a callback way. Thanks.

Also I commented the code. Let me know if some thing is not clear. There is small explanation in the issue description as well.

marcaaron · 2021-06-21T17:27:34Z

Just leaving a comment that I won't be able to help out with this review since there are higher priority items for me to get to at the moment. If you need help please ask in the Slack channel. Also curious if @TomatoToaster wants to check this PR out since he recently edited these files and might understand how this stuff works.

I can't review this right now

TomatoToaster

Hi I just had some questions for clarifying. Just making sure I understand how this works correctly.

src/libs/Network.js

src/libs/ActiveClientManager/index.js

src/libs/Network.js

parasharrajat · 2021-06-23T13:31:12Z

Hi @TomatoToaster, I will get back to you with answers on these But I am sure about that flag that it is needed.

src/libs/ActiveClientManager/index.js

parasharrajat · 2021-08-02T20:57:36Z

Does anyone feel that my solution is worth merging by doing some changes and refactor, then please do let me know? otherwise, this PR can be closed. Thanks.

tgolen · 2021-08-02T21:56:19Z

I'll give this another look tomorrow and let you know if I have any additional feedback!

…

On Mon, Aug 2, 2021 at 2:57 PM Rajat Parashar ***@***.***> wrote: Does anyone feel that my solution is worth merging by doing some changes and refactor, then please do let me know? otherwise, this PR can be closed. Thanks. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2703 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJMAB67VTNMGWXFJKZ5KMTT24BEZANCNFSM44FUNWSA> .

parasharrajat · 2021-08-03T19:13:32Z

@tgolen any thoughts?

tgolen · 2021-08-03T19:17:39Z

Nope, I still have other things on my list to get to before I look at this today. Thanks for your patience!

src/libs/Network.js

tgolen · 2021-08-03T20:19:53Z

src/libs/Network.js

+            // b) User is online.
+            // c) requests are not already loaded,
+            // d) When there is at least one request
+            if (!ActiveClientManager.isClientTheLeader()


I am still a big fan of this suggestion based on the following arguments:

There is no hint anywhere that this method must be run after ActiveClientManager.isReady()

If isClientTheLeader() is ever called without .isReady() then it's always going to give a false result

This is setting a trap for anyone else that decides to use isClientTheLeader() in the future.

tgolen · 2021-08-03T20:22:01Z

src/libs/Network.js

+
+        // If we have a request then we need to check if it can be persisted in case we close the tab while offline.
+        // We filter persisted requests from the normal Queue to remove duplicates
+        networkRequestQueue = _.reject(networkRequestQueue, (request) => {


src/libs/Network.js

parasharrajat · 2021-08-03T20:25:21Z

If isClientTheLeader() is ever called without .isReady() then it's always going to give a false result

It is not correct. Only on page reload this gives the wrong result (Until Onyx sets the key). for later cases it works just fine.
That's why notifications are not broken in the app on the web.

bump

Sorry I didn't understand.

bump

Same for this.

I want to start a separate convo from the above discussion, and it's specifically about the use of a "fake merge" in order to trigger a callback. I'm concerned that this is a little bit of a "hack" and it's not clear what's really happening. Instead, I think it would be better like this:

Sure I will do that.

tgolen · 2021-08-03T20:40:15Z

It is not correct. Only on page reload this gives the wrong result (Until Onyx sets the key).

OK, thanks for explaining. I see I wasn't completely correct, but the trap is still there. For example: if I add code somewhere else that executes on page load, and only reference isClientTheLeader() without using .isReady() then I wouldn't get an accurate result.

Sorry I didn't understand.

Oh, sorry! "bump" is a common term we use that indicates there is an unanswered question that needs to be addressed.

Sure I will do that.

Thanks!

parasharrajat · 2021-08-06T21:05:49Z

Updated @tgolen.

parasharrajat · 2021-08-09T17:43:22Z

This is ready for another review. I can move to the callback-based onReady method if pointed to do so.

ATM this PR fixes two issues. Not sure of the prioirty.

parasharrajat · 2021-08-11T13:06:44Z

🙄 Others please share your valuable feedback.

parasharrajat · 2021-08-11T18:32:18Z

Sorry, I am being impatient here but see no reason to hold unless you tell me the reason.

tgolen

Thanks for making that change to the callbacks.

We are not really seeing eye-to-eye on isClientTheLeader() being asynchronous.

If @TomatoToaster and @joelbettner are absolutely fine with the current implementation, and they feel they understand it and could fix any race conditions with it (as opposed to just approving this PR because of review-fatigue), then I am OK merging this as-is.

tgolen · 2021-08-11T22:51:34Z

All yours @marcaaron

parasharrajat · 2021-08-11T23:01:38Z

We are not really seeing eye-to-eye on isClientTheLeader() being asynchronous.

I agree with the point. Just finding it difficult to handle in the ONYX.connect callback without too much boilerplate. I am ready to move forward with the current implementation as ClientManager usages do not have any such race condition. Maybe if we have any problems with this we can rethink the approach here.

marcaaron · 2021-08-12T01:08:34Z

I have to be honest... reviewing this is going to be a bit of a context switch for me at the moment. The changes still seem a bit hard to follow, but to be fair, the persisting of requests did not really fit cleanly into the design when I added it.

That said, I feel like it's very possible we will want to do something like a Phase 2 Refactor for the networking code. And that's probably a good time to think a bit more about how we want this to work and just do it right.

So yeah that's a long way of saying... it feels like persisting requests are in a somewhat broken state and if this fixes them then I'm 👍

parasharrajat · 2021-08-13T16:35:59Z

Thanks Marc. Not sure what is the next step.

TomatoToaster · 2021-08-13T16:52:12Z

Yeah I think we can merge this. I can't think of a more perfect way to fix right now this and I agree that we can revisit this on a stage 2 refactor. I'll also sign up to help out on fixing any bugs with these persisted requests going forward 👍🏾 and help out with the eventual refactor since I've got this context at least.

OSBotify · 2021-08-13T16:53:10Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-08-17T18:52:36Z

🚀 Deployed to staging by @TomatoToaster in version: 1.0.85-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

botify · 2021-08-25T17:34:13Z

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-01. 🎊

OSBotify · 2021-08-25T17:55:20Z

🚀 Deployed to production by @roryabraham in version: 1.0.86-11 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

botify · 2021-08-30T17:21:22Z

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-06. 🎊

fix: messages are repeated on page refresh & process queue only when …

55e80fd

…active client

parasharrajat requested a review from a team as a code owner May 5, 2021 19:42

MelvinBot requested review from joelbettner and removed request for a team May 5, 2021 19:43

parasharrajat commented May 5, 2021

View reviewed changes

marcaaron requested changes May 7, 2021

View reviewed changes

marcaaron reviewed May 7, 2021

View reviewed changes

Merge branch 'main' of github.com:Expensify/Expensify.cash into parar…

58967bd

…asharrajat/offline

parasharrajat force-pushed the pararasharrajat/offline branch from a971bad to ed60b40 Compare May 9, 2021 13:57

fix: messages are repeated across tabs

8289bad

parasharrajat force-pushed the pararasharrajat/offline branch from ed60b40 to 8289bad Compare May 9, 2021 14:00

parasharrajat commented May 10, 2021

View reviewed changes

marcaaron previously requested changes May 10, 2021

View reviewed changes

parasharrajat mentioned this pull request Jun 10, 2021

iOS - App crashes after turning off the internet connection #3485

Closed

parasharrajat added 2 commits June 20, 2021 02:11

pull latest changes

aebdb86

improve code readability & process request when user comes back online

a3e5150

TomatoToaster self-requested a review June 22, 2021 18:28

TomatoToaster reviewed Jun 23, 2021

View reviewed changes

src/libs/Network.js Outdated Show resolved Hide resolved

src/libs/ActiveClientManager/index.js Outdated Show resolved Hide resolved

src/libs/Network.js Show resolved Hide resolved

joelbettner reviewed Jun 23, 2021

View reviewed changes

src/libs/ActiveClientManager/index.js Outdated Show resolved Hide resolved

tgolen requested changes Aug 3, 2021

View reviewed changes

refactor

ddb3cbd

joelbettner approved these changes Aug 10, 2021

View reviewed changes

TomatoToaster approved these changes Aug 11, 2021

View reviewed changes

parasharrajat requested a review from tgolen August 11, 2021 18:52

tgolen approved these changes Aug 11, 2021

View reviewed changes

TomatoToaster merged commit d6ff523 into Expensify:main Aug 13, 2021

This was referenced Aug 13, 2021

Deploy Checklist: Expensify.cash 2021-08-11 #4554

Closed

Deploy Checklist: New Expensify 2021-08-17 #4701

Closed

parasharrajat mentioned this pull request Aug 31, 2021

[Network] Messages are sent twice #4214

Closed

5 tasks

fixed messages are repeated on page refresh & process queue only when client is leader #2703

fixed messages are repeated on page refresh & process queue only when client is leader #2703

Conversation

parasharrajat commented May 5, 2021 • edited Loading

Details

Fixed Issues

Tests / QA Steps

Tested On

Screenshots

Web

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat commented May 7, 2021

Choose a reason for hiding this comment

marcaaron May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat commented May 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat Aug 3, 2021 • edited Loading

Choose a reason for hiding this comment

parasharrajat commented May 10, 2021

OLD Architecture

Multi Client Case

Proposed Architecture

marcaaron commented Jun 7, 2021

parasharrajat commented Jun 8, 2021

joelbettner commented Jun 18, 2021

parasharrajat commented Jun 18, 2021

parasharrajat commented Jun 19, 2021

marcaaron commented Jun 21, 2021

TomatoToaster left a comment

Choose a reason for hiding this comment

parasharrajat commented Jun 23, 2021

parasharrajat commented Aug 2, 2021

tgolen commented Aug 2, 2021 via email

parasharrajat commented Aug 3, 2021

tgolen commented Aug 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat commented Aug 3, 2021 • edited Loading

tgolen commented Aug 3, 2021

parasharrajat commented Aug 6, 2021 • edited Loading

parasharrajat commented Aug 9, 2021 • edited Loading

parasharrajat commented Aug 11, 2021 • edited Loading

parasharrajat commented Aug 11, 2021

tgolen left a comment

Choose a reason for hiding this comment

tgolen commented Aug 11, 2021

parasharrajat commented Aug 11, 2021

marcaaron commented Aug 12, 2021

parasharrajat commented Aug 13, 2021

TomatoToaster commented Aug 13, 2021

OSBotify commented Aug 13, 2021

OSBotify commented Aug 17, 2021

parasharrajat commented May 5, 2021 •

edited

Loading

parasharrajat May 7, 2021 •

edited

Loading

marcaaron May 7, 2021 •

edited

Loading

parasharrajat Aug 3, 2021 •

edited

Loading

parasharrajat commented Aug 3, 2021 •

edited

Loading

parasharrajat commented Aug 6, 2021 •

edited

Loading

parasharrajat commented Aug 9, 2021 •

edited

Loading

parasharrajat commented Aug 11, 2021 •

edited

Loading